feat(lua): add sparse array encode controls by membphis · Pull Request #126 · api7/lua-qjson

membphis · 2026-05-31T15:24:32Z

Summary

add qjson.encode_sparse_array(convert, ratio, safe) as a lua-cjson compatible module-level getter/setter
make plain-table sparse detection honor configurable convert/ratio/safe thresholds
document sparse-array migration behavior and add Lua compatibility coverage

Closes #123

Tests

cargo test --release
cargo test --release --no-default-features
cargo test --features test-panic --release
make lint
DYLD_LIBRARY_PATH=$PWD/target/release LUA_PATH='./lua/?.lua;/usr/local/openresty/lualib/?.lua;/usr/local/openresty/lualib/?/init.lua;;' LUA_CPATH='./vendor/lua-cjson/?.so;./target/release/lib?.dylib;./target/release/lib?.so;./?.so;/usr/local/openresty/lualib/?.so;/usr/local/lib/lua/5.1/?.so;/usr/local/openresty/luajit/lib/lua/5.1/?.so' ~/.luarocks/bin/busted --lua=/opt/homebrew/bin/luajit tests/lua --lpath='./lua/?.lua'

Summary by CodeRabbit

New Features
- Added sparse-array configuration support compatible with lua-cjson, enabling control over how sparse arrays are encoded with configurable conversion, ratio, and safety thresholds.
Documentation
- Updated migration guide to document the new sparse-array configuration API, including behavior, defaults, and when excessive sparsity checks are applied.

coderabbitai · 2026-05-31T15:24:47Z

Warning

Review limit reached

@membphis, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 33 minutes and 3 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6fb2e4a5-7ab8-4409-b45e-56a0cdd3abd3

📥 Commits

Reviewing files that changed from the base of the PR and between 8e38628 and fd152b9.

📒 Files selected for processing (3)

docs/migrating-from-cjson.md
lua/qjson/table.lua
tests/lua/encode_cjson_compat_spec.lua

📝 Walkthrough

Walkthrough

This PR adds a qjson.encode_sparse_array(convert, ratio, safe) configuration API matching lua-cjson, enabling runtime control of sparse-array detection thresholds. The implementation introduces module-level state variables, validates arguments, integrates with the encoder's table classification logic, and includes comprehensive tests and documentation updates.

Changes

Sparse-array Configuration

Layer / File(s)	Summary
Sparse-array configuration state and API function `lua/qjson/table.lua`	Defines three module-level state variables (`ENCODE_SPARSE_CONVERT`, `ENCODE_SPARSE_RATIO`, `ENCODE_SPARSE_SAFE`) with defaults (false, 2, 10). The `encode_sparse_array(convert, ratio, safe)` function validates `ratio` and `safe` as non-negative integers, updates state from provided arguments, and returns the effective triplet in both getter and setter modes.
Encoder sparse-table classification logic `lua/qjson/table.lua`	Updates `classify_plain_table` to check `ENCODE_SPARSE_RATIO > 0` before rejecting sparse tables. When sparse conversion is enabled, excessively sparse arrays are returned as `"object"` type; otherwise the error is raised. Allows sparse array encoding when `ratio=0` disables the check.
Module-level API export `lua/qjson.lua`	Re-exports `_M.encode_sparse_array = _lazy.encode_sparse_array` for public access.
Sparse-array configuration test suite `tests/lua/encode_cjson_compat_spec.lua`	Adds 116 lines of tests with per-test state reset (`after_each`), verifying getter/setter semantics, argument validation, encoding behavior for sparse configurations, sparse-to-object conversion, ratio and safe threshold interactions, and confirmation that type hints and lazy arrays bypass sparse checks.
Migration guide and documentation updates `docs/migrating-from-cjson.md`	Adds "Supported sparse-array configuration" section documenting the API, defaults, and excessive-sparsity trigger logic (`ratio > 0` AND `max > safe` AND `max > count * ratio`); notes that state persists across worker lifetime. Updates checklist item 6 to remove sparse-array guidance, leaving only `cjson.new` references.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

api7/lua-qjson#108: Both PRs modify lua/qjson/table.lua to change sparse-array classification and validation using ENCODE_SPARSE_RATIO/ENCODE_SPARSE_SAFE constants, with corresponding test updates.

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name	Status	Explanation	Resolution
Security Check	❌ Error	encode_sparse_array() modifies module state before validating all args. Calls like encode_sparse_array(true, -1) corrupt state before the error, affecting subsequent requests in the worker.	Stage values in locals, validate all arguments, commit state only if validation succeeds. Prevents partial state corruption per the review comment's proposed fix.
E2e Test Quality Review	⚠️ Warning	Critical code bug: encode_sparse_array() updates module state before validating all args; failed validation leaves state partially corrupted, affecting subsequent encode() calls.	Stage values in locals first, validate all args, then commit state. Add test verifying atomicity: qjson.encode_sparse_array(true, -1) should fail without changing state.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat(lua): add sparse array encode controls' accurately summarizes the main change—adding a new API function for configuring sparse array encoding behavior.
Linked Issues check	✅ Passed	All PR objectives from issue `#123` are met: the encode_sparse_array(convert, ratio, safe) API is implemented as a getter/setter with correct semantics, classify_plain_table honors the configurable thresholds, documentation is updated, and comprehensive tests cover defaults, setters, conversion logic, and edge cases.
Out of Scope Changes check	✅ Passed	All changes are directly related to implementing the sparse array configuration feature: core implementation in table.lua, module export in qjson.lua, migration documentation, and Lua tests. No unrelated modifications were introduced.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch codex/issue-123-sparse-array-detection

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/migrating-from-cjson.md`:
- Line 187: Update the phrasing in the sentence "qjson now supports lua-cjson
style sparse-array controls via" to hyphenate the compound modifier as
"lua-cjson-style" so it reads "qjson now supports lua-cjson-style sparse-array
controls via"; locate and replace that exact substring in the
docs/migrating-from-cjson.md content.

In `@lua/qjson/table.lua`:
- Around line 629-645: The setter _M.encode_sparse_array currently assigns
module-level flags (ENCODE_SPARSE_CONVERT, ENCODE_SPARSE_RATIO,
ENCODE_SPARSE_SAFE) before validating all inputs so a failing validation (e.g.
validate_non_negative_integer) can leave partial state changes; change the
implementation to stage incoming values into locals (e.g. local new_convert,
new_ratio, new_safe), run validate_non_negative_integer on new_ratio/new_safe as
needed, and only if all validations pass assign the staged locals back to the
module-level ENCODE_SPARSE_CONVERT, ENCODE_SPARSE_RATIO, ENCODE_SPARSE_SAFE;
keep the existing too-many-arguments check and behavior of convert defaulting to
true when not false.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4d85b9bb-7380-4e64-89fe-7c028248f7e0

📥 Commits

Reviewing files that changed from the base of the PR and between 613b5aa and 8e38628.

📒 Files selected for processing (4)

docs/migrating-from-cjson.md
lua/qjson.lua
lua/qjson/table.lua
tests/lua/encode_cjson_compat_spec.lua

feat(lua): add sparse array encode controls

8e38628

coderabbitai Bot reviewed May 31, 2026

View reviewed changes

Comment thread docs/migrating-from-cjson.md Outdated

Comment thread lua/qjson/table.lua

fix(lua): make sparse array settings atomic

fd152b9

membphis merged commit c842041 into main May 31, 2026
16 checks passed

membphis deleted the codex/issue-123-sparse-array-detection branch May 31, 2026 15:58

coderabbitai Bot mentioned this pull request May 31, 2026

Optimize encode by reusing original JSON slices for unchanged materialized data #128

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(lua): add sparse array encode controls#126

feat(lua): add sparse array encode controls#126
membphis merged 2 commits into
mainfrom
codex/issue-123-sparse-array-detection

membphis commented May 31, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 31, 2026 •

edited

Loading

Review limit reached

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Pre-merge checks failed

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

membphis commented May 31, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Tests

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Pre-merge checks failed

❌ Failed checks (1 error, 1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

membphis commented May 31, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 31, 2026 •

edited

Loading